[Core] Throw an error when the OAI_CONFIG_LIST is missing.#1082
[Core] Throw an error when the OAI_CONFIG_LIST is missing.#1082qingyun-wu merged 8 commits intomainfrom
Conversation
…CONFIG_LIST, and it is missing.
afourney
requested review from
a team,
qingyun-wu,
rickyloynd-microsoft,
sonichi and
yiranwu0
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1082 +/- ##
===========================================
+ Coverage 30.81% 60.89% +30.08%
===========================================
Files 30 30
Lines 4037 4033 -4
Branches 915 964 +49
===========================================
+ Hits 1244 2456 +1212
+ Misses 2714 1322 -1392
- Partials 79 255 +176
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
All those errors are also from misconfigured tests.... they should have been detected long ago, but were masked by this very issue I am correcting here. |
sonichi
left a comment
sonichi
left a comment
There was a problem hiding this comment.
To pass the tests with this change, need to move the failing line into the try...except block above it.
…not try to load the config_list when skipping OAI tests.
Ok, I think I fixed the merge conflict. Please take a look. |
|
Sorry for another conflict. Hopefully not difficult to resolve. |
Ok, that one was easier :) |
rickyloynd-microsoft
left a comment
rickyloynd-microsoft
left a comment
There was a problem hiding this comment.
Looks great!
* update openai model support * new gpt3.5 * docstr * function_call and content may co-exist * test function call --------- Co-authored-by: Qingyun Wu <qingyun.wu@psu.edu>
…#1082) * Throw an explicit and proper error when someone asks to load the OAI_CONFIG_LIST, and it is missing. * Updated to use pytest.raises. Added docstring. Updated some tests to not try to load the config_list when skipping OAI tests. * Fixed wrong indentation in config_list_from_json, and updated test_utils to work with non-empty lists. * Read key location from global constants. * Added missingpath. * Moved config_list_from_json to inside a skip check.
5 participants
Why are these changes needed?
We silently return an empty list when someone explicitly asks for an OAI_CONFIG_LIST and it can't be found. Paired with other default behavior (e.g, #1077) this can lead to costly calls to GPT-4.
Tackling this was actually one of the very first commits I pushed to autogen in #174. At the time, I was advised that changing default behavior was unwise and that I should perhaps just output an warning message.
Given #1077, and countless other issue since (maybe, #1025), I think it's about time that I go with my original instinct, and correct this unexpected and potentially dangerous behavior.
Related issue number
#1077, #1076, #1025, #174, #178, and so so many other.
Checks